- 
                Notifications
    You must be signed in to change notification settings 
- Fork 185
GC#DrawImage(image, destX, destY, destWidth, destHeight) now tries to load images at the requested destination size before drawing #2526
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
01766e0    to
    fa4a774      
    Compare
  
    8cf1bfb    to
    063d70b      
    Compare
  
    063d70b    to
    a9b9404      
    Compare
  
    82c03f5    to
    daf247e      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I scanned the PR and commented on some obvious or essential points I came across.
        
          
                bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/internal/image/FileFormat.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/GC.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
      e7af9a1    to
    d25c48a      
    Compare
  
            
          
                bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
      0e9ab55    to
    d3575ff      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the latest cleanups. They made the code much easier to understand.
Reading the code again I see that, if I am not mistaken, it breaks the existing (implicit) contract for the cache image data, such that it would like lead to a regression. That's why I request changes to avoid that we break existing behavior,
In addition, all the changes related to creating a temporary handle based on zoom for executing a drawing operation on look far more complicated than they need to be to me. I wrote a longer comment trying to explain what in my opinion can/should be done to improve that.
        
          
                ...les/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/graphics/ImageDataAtSizeProvider.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                ...les/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/graphics/ImageDataAtSizeProvider.java
          
            Show resolved
            Hide resolved
        
              
          
                bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/Image.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
      30de03f    to
    fe7048d      
    Compare
  
    3875d39    to
    d07939a      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good to me and I didn't find any regression testing it with the Runtime or the CustomControlExample that already use the new GC API that triggers the changed logic
d07939a    to
    db2d773      
    Compare
  
    8c1661a    to
    627d661      
    Compare
  
    …en possible. The DrawImage API with just destination coordinates now tries to load images at the requested size. If the requested size is not available, it falls back to the closest available size by calculating the appropriate zoom. Images are loadable at the target size if the image is created from an SVG stream or filename, or The image is created with an ImageDataProvider that is ImageDataAtSizeProvider. This commit also introduces the ImageDataAtSizeProvider interface to support dynamic sizing. Co-authored-by: Michael Bangas <michael.bangas@vector.com>
627d661    to
    b238dec      
    Compare
  
    This commit reverts the changes introduced in eclipse-platform#2526 where getImageData and getScaledImageData were mistakenly interchanged. The initial approach aimed to use this method in loadImageDataClosestTo when images couldn't be loaded at the exact size. However, the final implementation used loadImageData(zoom) instead, making those changes unnecessary. These leftover modifications were an unintended side effect, and this commit removes them.
This commit reverts the changes introduced in #2526 where getImageData and getScaledImageData were mistakenly interchanged. The initial approach aimed to use this method in loadImageDataClosestTo when images couldn't be loaded at the exact size. However, the final implementation used loadImageData(zoom) instead, making those changes unnecessary. These leftover modifications were an unintended side effect, and this commit removes them.
This commit reverts the changes introduced in eclipse-platform#2526 where getImageData and getScaledImageData were mistakenly interchanged. The initial approach aimed to use this method in loadImageDataClosestTo when images couldn't be loaded at the exact size. However, the final implementation used loadImageData(zoom) instead, making those changes unnecessary. These leftover modifications were an unintended side effect, and this commit removes them.
The DrawImage API with just destination coordinates now tries to load images at the requested size.
If the requested size is not available, it falls back to the closest available size by calculating the appropriate zoom.
Images are loadable at the target size if the image is created from an SVG stream or filename, or The image is created with an ImageDataProvider that is ImageDataAtSizeProvider.
This commit also introduces the ImageDataAtSizeProvider interface to support dynamic sizing.
With this changes we need to test the following
Steps to reproduce:
snippet 1 to check drawing svgs at custom sizes (click to show snippet)
**Step 1**: Copy the eclipse.svg to same path where the snippets are and run the below snippet. **Step 2**: Run the snippetpackage org.eclipse.swt.snippets;
before
after
**snippet 2** Drawing images with ImageDataAtSizeProvider
package org.eclipse.swt.snippets;
This depends on #2514 and #2509, Only last commit is relevant to this PR